Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jupytext depends on markdownitpy #601

Merged
merged 1 commit into from
Aug 30, 2020
Merged

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented Aug 30, 2020

This is an alternative to #599 which has the dependency on markdown-it-py only when Python >= 3.6.

@codecov
Copy link

codecov bot commented Aug 30, 2020

Codecov Report

Merging #601 into master will decrease coverage by 0.03%.
The diff coverage is 91.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
- Coverage   99.00%   98.97%   -0.04%     
==========================================
  Files          90       90              
  Lines        8980     9000      +20     
==========================================
+ Hits         8891     8908      +17     
- Misses         89       92       +3     
Impacted Files Coverage Δ
jupytext/myst.py 96.71% <91.48%> (-0.70%) ⬇️
jupytext/reraise.py 25.00% <0.00%> (-50.00%) ⬇️
jupytext/formats.py 96.91% <0.00%> (+0.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb697ef...883428d. Read the comment docs.

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Aug 30, 2020

Yep that looks fine thanks. Two minor things:

As you see in the CI, if you make a PR to master from an "internal" branch you get duplication of runs. To remove this we use:

on:
  push:
    branches: [master]
    tags:
      - '*'
  pull_request:

I made #602, to also update the import error message and documentation

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Aug 30, 2020

Another FYI:
I have recently found using tox to be really helpful for not having to manually create/maintain development environment (given that I'm working on over 10 different repos!) and have added them to most projects

Here's the tox.ini I'm using for jupytext:

# To use tox, see https://tox.readthedocs.io
# Simply pip or conda install tox
# If you use conda, you may also want to install tox-conda
# then run `tox` or `tox -- {pytest args}`
# To run in parallel using `tox -p` (this does not appear to work for this repo)

# To rebuild the tox environment, for example when dependencies change, use
# `tox -r`

# Note: if the following error is encountered: `ImportError while loading conftest`
# then then deleting compiled files has been found to fix it: `find . -name \*.pyc -delete`

[tox]
envlist = py{35,36,37,38}

[testenv:py{35,36,37,38}]
; recreate = false
deps = -rrequirements-dev.txt
commands = pytest {posargs}

[testenv:docs-{update,clean}]
deps = -rdocs/doc-requirements.txt
whitelist_externals = rm
commands =
    clean: rm -rf docs/_build
    sphinx-build {posargs} -nW --keep-going -b html docs/ docs/_build/html

@mwouts
Copy link
Owner Author

mwouts commented Aug 30, 2020

Thank you @chrisjsewell ! This is very helpful. I'll integrate this PR, as well as your update on the documentation and warnings, and open a follow-up issue to keep track of your multiple suggestions 😄 .

@mwouts mwouts mentioned this pull request Aug 30, 2020
5 tasks
@mwouts mwouts force-pushed the jupytext_depends_on_markdownitpy branch from 0e2f1c7 to 883428d Compare August 30, 2020 15:24
@mwouts mwouts merged commit cf992bd into master Aug 30, 2020
@mwouts mwouts deleted the jupytext_depends_on_markdownitpy branch August 30, 2020 15:47
@chrisjsewell
Copy link
Contributor

Thanks @mwouts 😄
No rush, but when can we expect a v1.6?

@mwouts
Copy link
Owner Author

mwouts commented Aug 30, 2020

Well if everything goes well, it could come within one day or two... fingers crossed!

@mwouts
Copy link
Owner Author

mwouts commented Aug 30, 2020

A release candidate is available here:

pip install jupytext==1.6.0rc0

@chrisjsewell
Copy link
Contributor

awesome thanks 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants